-
Notifications
You must be signed in to change notification settings - Fork 33
Allow limited public assembler builds on docs-builder #2379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs
Fixed
Show fixed
Hide fixed
src/services/Elastic.Documentation.Assembler/Building/AssemblerBuilder.cs
Fixed
Show fixed
Hide fixed
src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs
Fixed
Show fixed
Hide fixed
src/services/Elastic.Documentation.Assembler/Sourcing/RepositorySourcesFetcher.cs
Fixed
Show fixed
Hide fixed
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Mpdreamz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some changes requested.
.github/workflows/preview-build.yml
Outdated
| env: | ||
| AWS_RETRY_MODE: standard | ||
| AWS_MAX_ATTEMPTS: 6 | ||
| ASSEMBLER_PREVIEW_PATH_PREFIX: ${{ github.repository }}/assembled-docs/${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ASSEMBLER_PREVIEW_PATH_PREFIX: ${{ github.repository }}/assembled-docs/${{ github.event.pull_request.number }} | |
| ASSEMBLER_PREVIEW_PATH_PREFIX: ${{ github.repository }}/docs/${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
src/services/Elastic.Documentation.Assembler/Building/SitemapBuilder.cs
Outdated
Show resolved
Hide resolved
…nto feature/db-assembler-previews
Mpdreamz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit, rest LGTM!
src/services/Elastic.Documentation.Assembler/Building/SitemapBuilder.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also needs a cleanup
Problem Overview
Certain changes in docs-builder that are best viewed from an assembler build perspective can benefit from the collective review from writers, but it's not always straightforward to share specific links, show regressions, etc.
Allowing a limited assembler preview for docs-builder pull requests would allow for earlier and broader QA, and faster communication during feature development.
Proposed solution
This PR adds a new workflow to docs-builder that builds an assembler preview to a given PR #.
It contains the following limitations:
Other changes
llms.txt,sitemap.xmlandlink-index.snapshot.jsongeneration relied on hardcoded paths, which were incompatible with the previews introduced here. If there is a path prefix available, we now make use of it.